@swift-ci please smoke test

The new macro aliasing uncovered a latent issue where we would attempt to perform an optional-to-optional conversion on a type that is non-optional though may be aliased to an optional.

Could you explain this further?

The new macro aliasing uncovered a latent issue where we would attempt to perform an optional-to-optional conversion on a type that is non-optional though may be aliased to an optional.

Could you explain this further?

It was looking through a Optional<CVaList> to CVaList which is an Optional<UnsafeRawPointer> IIRC. This was just failing to convert Optional<Optional<UnsafeRawPointer>> to a CanType which is not optional.

I'm a bit concerned that this double-Optional situation doesn't seem like it should be happening in the first place. You say in the comment that "CVaList is sometimes an optional pointer", but AFAICT there is no CVaList type anywhere in the codebase. There is CVaListPointer, which is designated in MappedTypes.def as the type we map va_list and its various implementation-specific manifestations to. Since va_list is canonically an array typedef, it makes sense that it would manifest as a pointer when used in function parameters, and if that pointer is nullable in C we would import it as an Optional, but it isn't clear to me that we should be ending up with a double Optional here.

When you say that "a non-optional though may be aliased to an optional", what "aliasing" mechanism are we talking about here? Historically, I know the Clang importer logic driven by MAP_STDLIB_TYPE in MappedTypes.def is sensitive to the particular typedef used in the C header. If we recently added a mechanism to treat macros as typedefs, is that somehow causing the Clang importer to use a different translation? Should we fix this problem there instead of trying to cope with the consequences in SILGen?

Sorry, I did mean CVaListPointer. The aliasing mechanism is the CPP. Consider the following:

extern void functionX(void);
#define function functionX

This is now imported as:

var function: @convention(c) () -> Void {
  functionX
}

This was added recently, and I don't think that the import from the ClangImporter is being worked around here. This is fixing an issue that was occurring in ICU where we see this:

#define u_vformatMessage swift_u_vformatMessage
int32_t u_vformatMessage(const char *, const UChar *, int32_t, UChar, int32_t, va_list, UErrorCode);

This is getting imported into Swift as:

var u_vformatMessage: ... {
  swift_u_vformatMessage
}

Then when trying to create the va_list for a call like the following, it would abort due to the CanType for conversion not being optional on the output.

withVaList([0.0]) {
  u_vformatMessage(..., $0, ...)
}

ManagedValue
SILGenFunction::emitOptionalToOptional(SILLocation loc,
                                       ManagedValue input,
                                       SILType resultTy,
                                       ValueTransformRef transformValue,
                                       SGFContext C) {
  ...
  SILType noOptResultTy = resultTy.getOptionalObjectType();
  assert(noOptResultTy);  // <---- this would previously trip

@swift-ci please smoke test

but it isn't clear to me that we should be ending up with a double Optional here

I don't think that we are seeing a double optional, but the type check treats the CVaListPointer as an optional type in the type system, which is why I was writing it as a double optional.

@jckarter - any more concerns? I'd like to get this in soon

It still sounds like something fishy is going on here, but the change doesn't seem harmful and doesn't seem to break any tests, so I don't object to you landing this patch in the meantime.

@jckarter sounds good! Is there anything I should file so that we can follow up on the investigation? Or is there something specific that would help identify a potentially better longer term solution?